-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Port MASTG-TEST-0026: Testing Implicit Intents (android) (by @appknox) #3271
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
@cpholguera Please review. This format is slightly different, so please let me know the changes that are needed. |
TheDauntless
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @ScreaMy7 ! Thanks for contributing!
This PR is mixing up two different things though. Implicit intents are an issue because they can be intercepted by other applications, thereby possible intercepting sensitive information. While the explanation seems to be correct, the demo itself doesn't show the vulnerability. Can you please update the code to showcase the vulnerability?
|
@TheDauntless Thanks for the review. Have you tried the dynamic demo, which has an attacker application to trigger the intent redirect to the internal parts of the vulnerable application? Could you please explain more about the mix-up? |
|
Either the explanation of the test is wrong, or the implementation. This test is about implicit intent interception so the demo would be:
Result: The user can choose between the legitimate app and the attacker app, and the sensitive data would leak if they choose the attacker app Your demo describes the attacker intercepting data, but data isn't intercepted anywhere? You're just triggering an implicit activity with an explicit intent; all the data stays inside of the vulnerableapp. Where is the attack? What you are describing is something that Android fixes itself in Android 15: https://developer.android.com/about/versions/15/behavior-changes-15#safer-intents |
|
Hi @TheDauntless , |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR ports the deprecated MASTG-TEST-0026 (Testing Implicit Intents) to MASTG V2 format by creating two new tests that cover different aspects of implicit intent vulnerabilities. The change maintains backward compatibility by marking the original test as deprecated while providing comprehensive coverage through static and dynamic analysis approaches.
- Deprecates MASTG-TEST-0026 and creates MASTG-TEST-0287 (static analysis) and MASTG-TEST-0286 (dynamic analysis)
- Adds comprehensive demonstration files showing both vulnerable code patterns and exploitation techniques
- Includes Semgrep rule for automated detection of exported activities with custom intent filters
Reviewed Changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/android/MASVS-CODE/MASTG-TEST-0026.md | Marks original test as deprecated with references to new tests |
| tests-beta/android/MASVS-CODE/MASTG-TEST-0287.md | New static analysis test for implicit intent vulnerabilities |
| tests-beta/android/MASVS-CODE/MASTG-TEST-0286.md | New dynamic analysis test for implicit intent interception |
| rules/mastg-android-custom-intent-intecept.yml | Semgrep rule for detecting vulnerable intent filter configurations |
| demos/android/MASVS-CODE/MASTG-DEMO-0059/ | Static analysis demonstration with Semgrep output |
| demos/android/MASVS-CODE/MASTG-DEMO-0058/ | Dynamic analysis demonstration with vulnerable and attacker code |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| @@ -0,0 +1,28 @@ | |||
| --- | |||
| title: Implicit intent to intecept internal app components | |||
Copilot
AI
Aug 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a typo in the title: 'intecept' should be 'intercept'.
| title: Implicit intent to intecept internal app components | |
| title: Implicit intent to intercept internal app components |
| @@ -0,0 +1,2 @@ | |||
| # shellcheck disable=SC2148 | |||
| NO_COLOR=true semgrep -c ../../../../rules/mastg-android-custom-intent-filter-intercept.yml ../MASTG-DEMO-0058/AndroidManifest_reversed.xml --text -o output.txt No newline at end of file | |||
Copilot
AI
Aug 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The referenced rule file 'mastg-android-custom-intent-filter-intercept.yml' doesn't match the actual filename 'mastg-android-custom-intent-intecept.yml'. This will cause the semgrep command to fail.
| NO_COLOR=true semgrep -c ../../../../rules/mastg-android-custom-intent-filter-intercept.yml ../MASTG-DEMO-0058/AndroidManifest_reversed.xml --text -o output.txt | |
| NO_COLOR=true semgrep -c ../../../../rules/mastg-android-custom-intent-intecept.yml ../MASTG-DEMO-0058/AndroidManifest_reversed.xml --text -o output.txt |
|
|
||
| Let's run our @MASTG-TOOL-0110 rule against the manifest file and code. | ||
|
|
||
| {{ ../../../../rules/mastg-android-custom-intent-filter-intercept.yml }} |
Copilot
AI
Aug 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The referenced rule file path 'mastg-android-custom-intent-filter-intercept.yml' doesn't match the actual filename 'mastg-android-custom-intent-intecept.yml'. This will result in a broken reference.
| {{ ../../../../rules/mastg-android-custom-intent-filter-intercept.yml }} | |
| {{ ../../../../rules/mastg-android-custom-intent-intecept.yml }} |
| @@ -0,0 +1,25 @@ | |||
| <manifest xmlns:android="http://schemas.android.com/apk/res/android" | |||
| package="com.example.datainteceptor"> | |||
Copilot
AI
Aug 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a typo in the package name: 'datainteceptor' should be 'datainterceptor' to match the correct spelling of 'interceptor'.
| package="com.example.datainteceptor"> | |
| package="com.example.datainterceptor"> |
| @@ -0,0 +1,15 @@ | |||
| package com.example.datainteceptor | |||
Copilot
AI
Aug 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a typo in the package name: 'datainteceptor' should be 'datainterceptor' to match the correct spelling of 'interceptor'.
| package com.example.datainteceptor | |
| package com.example.datainterceptor |
| @@ -0,0 +1,24 @@ | |||
| package com.example.datainteceptor | |||
Copilot
AI
Aug 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a typo in the package name: 'datainteceptor' should be 'datainterceptor' to match the correct spelling of 'interceptor'.
| package com.example.datainteceptor | |
| package com.example.datainterceptor |
bernhste
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for this demo. This is a nice addition to the implicit intent risks. Let's take this PR to think about how to cover them properly.
In the MAST-V1 test, there are additional examples that illustrate the fundamental issue, and I think this PR should not deprecate them without covering the general issue.
So I provided a suggestion of how we could write the tests and demos based on this PR. Please let me know what you think about it.
We can also pick this broader discussion in #2997.
| languages: | ||
| - xml | ||
| metadata: | ||
| summary: The rule checks if the implicit Intent is used to start any activity without specifying the target component |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| summary: The rule checks if the implicit Intent is used to start any activity without specifying the target component | |
| summary: The rule checks if the implicit Intent is used to start any activity without specifying the target component | |
| cwe: "CWE-927: Use of Implicit Intent for Sensitive Communication" |
Since there is an explicit CWE for this weakness, I suggest that we add it here.
|
|
||
| A convenient way to dynamically test for implicit intents, especially to identify potentially leaked sensitive data, is to use Frida or frida-trace and hook the `startActivityForResult` and `onActivityResult` methods and inspect the provided intents and the data they contain. | ||
|
|
||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove the trailing blank line.
| platform: android | ||
| id: MASTG-TEST-0286 | ||
| type: [dynamic] | ||
| weakness: MASWE-0083 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| weakness: MASWE-0083 | |
| weakness: MASWE-0066 |
MASWE-0066 maps to CWE-927 which is "Use of Implicit Intent for Sensitive Communication".
| platform: android | ||
| id: MASTG-TEST-0287 | ||
| type: [static] | ||
| weakness: MASWE-0083 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| weakness: MASWE-0083 | |
| weakness: MASWE-0066 |
MASWE-0066 maps to CWE-927 which is "Use of Implicit Intent for Sensitive Communication".
| @@ -0,0 +1,42 @@ | |||
| --- | |||
| platform: android | |||
| title: Implicit Intent Hijacking with same custom intent filter | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| title: Implicit Intent Hijacking with same custom intent filter | |
| title: Implicit Intent Hijacking with Same Custom Intent Filter |
Title capitalization.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This demo implements a special case where the developer mistakenly uses an implicit intent and an exported Activity for supposedly internal ICP. This is generally not the intended use case of implicit intents, as they should be used when the developers do not know which app should react to it. I think this should be more clearly described in the demo.
Further, the issue of implicit intent hijacking is more general, as actions can be anything from built-in to the same package name or completely custom. And depending on the configuration, the likelihood of an attack varies. This demo declares two intent filters for the custom action. This means the user must, in any case, choose between the valid and the malicious app:
I think this should be mentioned in the demo because it is relevant to the risk.
To cover the different situations, we should make different demos. Let me suggest the following:
- MASTG-DEMO-0058: Dynamic Detection of Implicit Intent Hijacking
- MASTG-DEMO-0059: Static Detection of External Intent Misuse for Internal IPC
- MASTF-TEST-xxxx: Dynamic Detection of Arbitrary File Read Using Implicit Intent Hijacking
- MASTF-TEST-xxxx: Dynamic Detection of Arbitrary Code Execution Implicit Intent Hijacking
We take your proposal for the first two but implement the two examples from the MASTG-V1-Test as new demos so we don't lose them.
These demos should also demonstrate situations where no app chooser is shown to the user. For example, if an implicit intent is used for which there is only the attacker app available. I will propose this in the discussion #2997.
I made a fork of the PR with a suggestion for updates to the first two demos: https://github.com/bernhste/owasp-mastg/tree/TEST-0026-draft-updated-DEMO
Please take a look and let me know what you think. I'm also happy to implement additional demos.
| class MastgTest (private val context: Context){ | ||
|
|
||
| fun mastgTest(): String { | ||
| val sensitiveString = "Hello from the OWASP MASTG Test app." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the placeholder string from the template app. I would suggest using the GUI to inform the user about the status of the test. Something like that:
// Launch implicit intent - any app can intercept this
try {
context.startActivity(vulnerableIntent)
r.add(Status.FAIL, "Hijackable implicit intent launched")
} catch (e: Exception) {
r.add(Status.ERROR, e.toString())
}There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I recommend renaming this test to "Testing for Implicit Intent Hijacking" and describing how to generally test for outgoing implicit intents (MASWE-0066 with the restriction of CWE-927).
This should be a dynamic test, which uses Frida.re to intercept outgoing intents and tests if they are implicit or not. They should hook the following methods and dynamically assess whether the intent is implicit or not:
startActivity(Intent)startActivityForResult(Intent, int)startActivities(Intent[])startService(Intent)sendBroadcast(Intent)
A basis for the script may be: https://codeshare.frida.re/@ivan-sincek/android-intent-monitor
They do not exist yet, but I'm happy to provide a suggestion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would rename this test to "Testing for Exported Activities With Custom Intent Filter Actions" because the semgrep rule does that.
It should be clear that a failed test can indicate that a risk exists. But to evaluate the risk, we have to either manually do static analysis or dynamic analysis using MAST-TEST-0286.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should make sure not to lose the demos from this test. As written in the comment of MASTG-DEMO-0058.md, we could make dedicated demos for these cases.
closes #2997